feat/light-mode #444
feat/light-mode #444
Conversation
WalkthroughAdds a data-theme driven theming system and theme toggle, wires theme through shell/header and several consumers, introduces hideGradient/articleCard and imagePriority inputs, updates many templates/styles for light/dark tokens, and adjusts Tailwind config and dependencies. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Header as HeaderComponent
participant Shell as RootShellComponent
participant Store as AppThemeStore
participant DOM as Document(:root)
participant Consumer as Component (AboutUs / Giscus / Cards)
User->>Header: click theme toggle
Header-->>Shell: emit themeToggle
Shell->>Store: toggleTheme()
Store->>DOM: set data-theme attribute (dark|light)
Store->>Store: patchState(theme)
Store-->>Consumer: theme() signal updates
note right of Consumer: Consumers read theme() and adjust hideGradient, classes, giscus theme, images
Estimated code review effort🎯 4 (Complex) | ⏱️ ~70 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
PR is detected, will deploy to dev environment |
|
Deployed to dev environment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
libs/blog/become-author/feature-become-author-page/src/lib/become-author-page/become-author-page.component.html (2)
34-34: Consider using theme-aware text color for improved theme compatibilityWhile white text works well on the pink background, if the button background color might change based on theme, consider using a theme-aware text color class.
If the button background remains pink in all themes, this change is fine. Otherwise, consider:
- <p class="text-sm font-bold text-white"> + <p class="text-sm font-bold text-al-pink-foreground">This assumes you have defined appropriate foreground colors for your themed backgrounds.
64-64: Consider using theme-aware text color for improved theme compatibilitySimilar to the other button, if the background color remains pink in all themes, white text is appropriate. For full theme support, consider a variable-based approach.
If the button background remains pink in all themes, this change is fine. Otherwise, consider using a theme-aware text color that complements the button background.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
apps/blog/src/assets/icons/arrow-down.svgis excluded by!**/*.svgpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (30)
libs/blog/about-us/feature-about-us/src/lib/feature-about-us/feature-about-us.component.html(2 hunks)libs/blog/articles/feature-latest-articles/src/lib/feature-latest-articles/feature-latest-articles.component.ts(0 hunks)libs/blog/articles/ui-article-card/src/lib/components/article-compact-card/article-compact-card.component.html(1 hunks)libs/blog/articles/ui-article-card/src/lib/components/article-compact-card/article-compact-card.component.ts(2 hunks)libs/blog/articles/ui-article-card/src/lib/components/article-hero-card/article-hero-card-skeleton.component.ts(1 hunks)libs/blog/articles/ui-article-card/src/lib/components/article-horizontal-card/article-horizontal-card.component.html(1 hunks)libs/blog/articles/ui-article-card/src/lib/components/article-regular-card/article-regular-card.component.html(3 hunks)libs/blog/articles/ui-article-card/src/lib/ui-article-card/ui-article-card.component.html(1 hunks)libs/blog/articles/ui-article-list-title/src/lib/ui-article-list-title/ui-article-list-title.component.html(1 hunks)libs/blog/authors/ui-author-card/src/lib/author-card/author-card-template.component.ts(2 hunks)libs/blog/authors/ui-author-card/src/lib/author-card/author-card.component.html(1 hunks)libs/blog/become-author/feature-become-author-page/src/lib/become-author-page/become-author-page.component.html(2 hunks)libs/blog/become-author/feature-become-author-page/src/lib/components/become-author-list-item/become-author-list-item.component.html(1 hunks)libs/blog/layouts/ui-layouts/src/lib/footer/components/footer-logo.component.ts(2 hunks)libs/blog/layouts/ui-layouts/src/lib/footer/components/footer-social-media-icons.component.ts(1 hunks)libs/blog/layouts/ui-layouts/src/lib/footer/footer.component.html(1 hunks)libs/blog/layouts/ui-layouts/src/lib/footer/footer.component.scss(1 hunks)libs/blog/layouts/ui-navigation/src/lib/navigation/navigation.component.html(3 hunks)libs/blog/layouts/ui-navigation/src/lib/navigation/navigation.component.ts(1 hunks)libs/blog/newsletter/feature-newsletter/src/lib/feature-newsletter/newsletter.component.html(1 hunks)libs/blog/partners/ui-partners/src/lib/partners-list/partners-list.component.html(1 hunks)libs/blog/shared/ui-card/src/lib/card.component.ts(2 hunks)libs/blog/shared/ui-card/src/lib/card.stories.ts(1 hunks)libs/blog/shared/ui-difficulty/src/lib/ui-difficulty.component.ts(3 hunks)libs/blog/shared/ui-pill/src/lib/pill.directive.ts(1 hunks)libs/blog/shared/ui-social-media-icons/src/lib/social-media-icons.component.html(1 hunks)libs/blog/writing-rules/feature-writing-rules/src/lib/rules-row/rules-row.component.html(2 hunks)libs/shared/assets/src/lib/styles/main.scss(1 hunks)package.json(1 hunks)tailwind.preset.js(2 hunks)
💤 Files with no reviewable changes (1)
- libs/blog/articles/feature-latest-articles/src/lib/feature-latest-articles/feature-latest-articles.component.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
libs/blog/articles/ui-article-card/src/lib/components/article-compact-card/article-compact-card.component.ts (1)
libs/blog/shared/types/src/lib/article-card.ts (1)
ArticleCard(1-13)
🔇 Additional comments (48)
package.json (1)
67-67: Verify plugin version compatibility
The plugin is added as a beta release; please confirm that2.0.0-beta.0oftailwindcss-theme-variantsis compatible with Tailwind CSS^3.3.6and appropriate for production use.libs/blog/shared/ui-card/src/lib/card.stories.ts (1)
61-61: Demonstration of transparent background variant
This story correctly showcases the transparent background use case by applyingbg-transparentto the<al-card>element.libs/blog/newsletter/feature-newsletter/src/lib/feature-newsletter/newsletter.component.html (1)
8-8: Consistent use of primary-foreground token
Applyingtext-al-primary-foregroundensures that the heading color adapts to theme variants and remains consistent across light/dark modes.libs/blog/articles/ui-article-card/src/lib/components/article-hero-card/article-hero-card-skeleton.component.ts (1)
10-10: Consistent transparent background
Addingbg-transparentaligns the skeleton card with the new light-mode theming, ensuring the skeleton blends correctly across themes.libs/blog/articles/ui-article-card/src/lib/components/article-horizontal-card/article-horizontal-card.component.html (1)
3-3: Verify removal of fixed height
Removingmax-h-52in favor of theme-based styling could affect the card's vertical sizing. Please verify that the card height remains consistent and does not overflow its container across breakpoints.libs/blog/authors/ui-author-card/src/lib/author-card/author-card.component.html (1)
45-45: Apply theme-aware text color
Addingtext-al-foregroundensures the author name responds to light/dark mode variants and aligns with the new theming tokens. Looks good.libs/blog/partners/ui-partners/src/lib/partners-list/partners-list.component.html (1)
6-6: Verify dark mode background color
Withbg-whitestill applied, this element may not adapt its background in dark mode. Please confirm thatbg-whiteis intended, or consider using a theme-variant class (e.g.,dark:bg-al-card) so the background aligns with dark theme.libs/blog/articles/ui-article-list-title/src/lib/ui-article-list-title/ui-article-list-title.component.html (1)
4-4: Standardize primary heading color
Addingtext-al-primary-foregroundensures the title adheres to the new primary text color token and is consistent with other headings across the app.libs/blog/articles/ui-article-card/src/lib/ui-article-card/ui-article-card.component.html (1)
15-18: LGTM! Improved consistency across card variantsAdding the
imagePriorityinput to the compact card variant brings consistency with the other card types and enables image loading optimization.libs/blog/writing-rules/feature-writing-rules/src/lib/rules-row/rules-row.component.html (2)
14-14: Good addition of explicit text colorAdding the
text-whiteclass ensures the step numbers maintain proper contrast with the pink background in both light and dark modes.
49-49: Proper contrast for button textThe addition of
text-whiteensures the button text has adequate contrast against the pink background regardless of theme, supporting accessibility.libs/blog/about-us/feature-about-us/src/lib/feature-about-us/feature-about-us.component.html (3)
2-2: Good use of semantic color tokenUsing
text-al-primary-foregroundfor the title enables proper theme adaptation instead of a hard-coded color value.
6-6: Appropriate background setting for theme supportSetting the card to
bg-transparentallows it to properly inherit background colors when switching between light and dark themes.
38-38: Consistent use of semantic color tokenUsing
text-al-primary-foregroundfor the authors title maintains styling consistency with the main title and supports theme adaptation.libs/blog/layouts/ui-navigation/src/lib/navigation/navigation.component.html (3)
9-9: Improved layout with centered navigationAdding
justify-centerimproves the horizontal navigation layout by properly centering the navigation items.
24-25: Good implementation of theme-aware text stylingReplacing static text color with conditional styling
[ngClass]="whiteFont() ? 'text-white' : 'text-al-foreground'"enables proper text color adaptation based on context and theme.Make sure the
whiteFont()input property is properly implemented in the component TypeScript file and documented for consumers of this component.
37-38: Consistent styling for external linksThe same conditional text styling is correctly applied to external links, maintaining styling consistency across all navigation items.
libs/blog/layouts/ui-layouts/src/lib/footer/components/footer-logo.component.ts (1)
2-2: Good component simplificationRemoving the unused
inputimport and thecurrentYearproperty simplifies the component appropriately. This cleanup removes unnecessary functionality while maintaining the component's core purpose.Ensure that any components using
<al-footer-logo>no longer pass thecurrentYearinput property, as it has been removed.Also applies to: 23-23
libs/blog/become-author/feature-become-author-page/src/lib/components/become-author-list-item/become-author-list-item.component.html (3)
3-5: Consistent trailing comma in[ngClass]object literal
The trailing comma after the single entry is valid and aligns with our formatting conventions for multiline object literals.
11-11: Explicittext-whiteclass applied for contrast
Addingtext-whiteensures the index label remains legible against the pink background.
15-15: Use oftext-al-pinktoken for the tick icon
Thetext-al-pinktoken aligns with our design system’s color palette.libs/blog/layouts/ui-layouts/src/lib/footer/footer.component.html (1)
5-6: Footer logo and navigation setup looks good
Explicitly centering the logo and passingwhiteFont="true"to<al-navigation>ensures consistent styling.libs/blog/layouts/ui-layouts/src/lib/footer/footer.component.scss (2)
15-17: Updated default grid-template-areas
The new two-row layout ('logo social'/'nav nav') is clear and concise for smaller viewports.
19-23: Verify breakpoint rename from@screen xlto@screen lg
Dropping the MD breakpoint and shifting the XL layout to LG may alter mid-size layouts. Please confirm that the updated breakpoints align with the design mockups and QA in all viewport widths.libs/shared/assets/src/lib/styles/main.scss (2)
11-17: New CSS variables for theming added
Introducing--primary-foregroundand--greycompletes the palette for both light and dark modes. This aligns with the Tailwind configuration’s new tokens.
19-29: Light-mode media query block implemented
Theprefers-color-scheme: lightoverride correctly defines lighter values for all variables, enabling automatic theme switching.libs/blog/shared/ui-card/src/lib/card.component.ts (2)
71-73: GradientCardDirective classes updated for theme variants
Switching to'bg-al-radial-gradient dark:bg-al-background'removes the border and adds a dark-mode fallback—this matches our updated theming strategy.
99-101: Ensure component and directivehostClassesmerge correctly
BothCardComponentand a consumer ofalGradientCardbind to theclassattribute. Please verify in the rendered DOM that classes from both the component and the directive are applied and not overridden.libs/blog/authors/ui-author-card/src/lib/author-card/author-card-template.component.ts (3)
11-11: Great conversion to standalone component!This conversion to a standalone component aligns with modern Angular best practices and improves the component's modularity.
23-24: Properly implemented light/dark mode stylingThe
alGradientCarddirective and conditional dark/light mode classes are correctly applied, ensuring consistent styling that adapts to the theme. The background styling appropriately uses different colors for each theme variant.
27-28: LGTM: Content styling with directiveAdding the
alCardContentdirective to the inner content maintains styling consistency with other card components in the application.libs/blog/articles/ui-article-card/src/lib/components/article-compact-card/article-compact-card.component.html (3)
3-3: Improved positioning and container stylingThe addition of
relative,overflow-hidden, andbg-blackclasses properly prepares the container for the new image layering approach.
6-13: Great improvement using proper image elementsReplacing CSS background with an actual
<img>element is a significant improvement that:
- Enhances accessibility with proper alt text
- Enables image optimization through NgOptimizedImage
- Provides better loading control with the priority attribute
- Makes explicit width/height handling possible
The absolute positioning with opacity control creates a nice background effect while maintaining content readability.
14-14: Proper text color handlingThe explicit text color setting ensures good contrast over the background image in all scenarios.
libs/blog/articles/ui-article-card/src/lib/components/article-compact-card/article-compact-card.component.ts (2)
1-1: Well-implemented standalone component conversionThe component has been properly converted to standalone with all necessary imports. Including NgOptimizedImage enables better image loading performance and optimization.
Also applies to: 13-20
25-25: Good addition of image priority controlThe new
imagePriorityinput property allows control over image loading priority, which is important for performance optimization and core web vitals. This aligns with the template changes using this property.libs/blog/articles/ui-article-card/src/lib/components/article-regular-card/article-regular-card.component.html (5)
3-3: Improved border styling for theme consistencyThe border styling now properly handles light and dark mode variations, maintaining visual separation in light mode while using transparent borders in dark mode.
21-25: Enhanced avatar component with priority loadingAdding the
[priority]binding to the avatar component improves loading performance for this important UI element.
26-28: Proper text color styling for author nameThe explicit text color class ensures consistent styling across light and dark modes.
41-47: Improved heading styling with proper color and text propertiesThe heading now uses explicit color, font weight, and style classes, ensuring consistent appearance across theme modes.
48-50: Consistent paragraph styling with theme-aware colorsThe paragraph styling now properly uses theme-specific colors with appropriate font styling.
tailwind.preset.js (4)
1-5: Excellent addition of theme variant supportGood job introducing the
tailwindcss-theme-variantsplugin which enables media query-based theme switching. This is a more modern approach than the class-based dark mode.
13-14: New color tokens look goodThe addition of 'al-primary-foreground' and 'al-grey' tokens will help maintain consistent theming across light and dark modes.
Also applies to: 22-22
35-35: Box shadow color change looks intentionalChanging from
--borderto--greyfor theal-full-bordershadow matches the new theming approach. Just ensure this change doesn't unexpectedly affect existing components.
39-51: Well-structured theme configurationThe plugin configuration properly sets up light and dark themes using media queries. This approach allows the UI to automatically adapt to the user's system preferences.
libs/blog/shared/ui-difficulty/src/lib/ui-difficulty.component.ts (3)
28-30: Good adaptation for light/dark mode in left blockThe conditional class application correctly handles both light and dark modes based on the
isColorBackground()condition:
- Using
dark:bg-al-background bg-transparentfor colored backgrounds- Using
bg-al-greyfor non-colored backgroundsThis approach maintains visual consistency across themes.
47-50: Well-implemented shadow handling for different themesThe shadow classes are properly updated for the dot container:
dark:shadow-al-full-background shadow-transparentfor colored backgroundsshadow-al-full-borderfor non-colored backgroundsThis ensures proper visual representation in both light and dark modes.
80-82: Consistent styling in right blockThe right block styling matches the left block's implementation, maintaining design consistency throughout the component. The code cleanly handles both light and dark themes.
| <h3 | ||
| id="social-media-title" | ||
| class="mb-4 hidden text-sm font-bold lg:block" | ||
| class="mb-4 hidden text-sm font-bold text-[#fff] lg:block" |
There was a problem hiding this comment.
Use design token instead of hardcoded hex
To support dynamic theming and maintain consistency with other components, replace the literal hex color with the text-al-primary-foreground token:
- class="mb-4 hidden text-sm font-bold text-[#fff] lg:block"
+ class="mb-4 hidden text-sm font-bold text-al-primary-foreground lg:block"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| class="mb-4 hidden text-sm font-bold text-[#fff] lg:block" | |
| class="mb-4 hidden text-sm font-bold text-al-primary-foreground lg:block" |
| variants: { | ||
| variant: <Record<AlPillVariant, string>>{ | ||
| flat: 'bg-al-primary/90', | ||
| flat: 'bg-al-primary/90 text-white', |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use design token for pill text color
For theme variant compatibility, replace text-white with the text-al-primary-foreground token in the flat variant:
- flat: 'bg-al-primary/90 text-white',
+ flat: 'bg-al-primary/90 text-al-primary-foreground',📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| flat: 'bg-al-primary/90 text-white', | |
| flat: 'bg-al-primary/90 text-al-primary-foreground', |
libs/blog/shared/ui-social-media-icons/src/lib/social-media-icons.component.html
Show resolved
Hide resolved
| }) | ||
| export class NavigationComponent { | ||
| readonly layout = input<'vertical' | 'horizontal'>('horizontal'); | ||
| readonly whiteFont = input<boolean>(false); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider a more flexible theming approach instead of a boolean flag
While adding a whiteFont input provides basic text color control, a more comprehensive theming approach would better support light mode implementation.
Consider using a theme enum or string input that supports multiple theme options:
- readonly whiteFont = input<boolean>(false);
+ readonly textTheme = input<'default' | 'light' | 'dark' | 'contrast'>('default');Then in the template, you could apply different classes based on the selected theme:
[ngClass]="{
'text-white': textTheme() === 'light',
'text-al-foreground': textTheme() === 'default',
'text-al-primary': textTheme() === 'dark',
'text-al-contrast': textTheme() === 'contrast'
}"This approach would be more maintainable as you add more theme variants beyond just light mode.
libs/blog/layouts/ui-layouts/src/lib/footer/footer.component.html
Outdated
Show resolved
Hide resolved
libs/blog/layouts/ui-layouts/src/lib/footer/footer.component.html
Outdated
Show resolved
Hide resolved
|
PR is detected, will deploy to dev environment |
|
Deployed to dev environment |
|
PR is detected, will deploy to dev environment |
|
Deployed to dev environment |
|
PR is detected, will deploy to dev environment |
|
Deployed to dev environment |
|
PR is detected, will deploy to dev environment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
libs/blog/app-theme/data-access-app-theme/src/app-theme.store.ts (1)
42-42: Consider addressing the architectural TODO.The TODO comment highlights a valid architectural concern about decoupling the store from
CCAppThemeConsumer. Consider creating an interface to improve testability and modularity.Would you like me to help design an interface to decouple the theme store from the consumer implementation?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
apps/blog/src/assets/icons/moon.svgis excluded by!**/*.svgapps/blog/src/assets/icons/sun.svgis excluded by!**/*.svg
📒 Files selected for processing (5)
libs/blog/app-theme/data-access-app-theme/src/app-theme.store.ts(3 hunks)libs/blog/layouts/ui-layouts/src/lib/header/header.component.ts(3 hunks)libs/blog/shared/ui-icon/src/lib/icon/icon.component.ts(1 hunks)libs/blog/shell/feature-shell-web/src/lib/root-shell.component.ts(4 hunks)libs/shared/assets/src/lib/styles/main.scss(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- libs/blog/shared/ui-icon/src/lib/icon/icon.component.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- libs/shared/assets/src/lib/styles/main.scss
🧰 Additional context used
🧬 Code Graph Analysis (1)
libs/blog/shell/feature-shell-web/src/lib/root-shell.component.ts (1)
libs/blog/app-theme/data-access-app-theme/src/app-theme.store.ts (1)
AppThemeStore(11-34)
🔇 Additional comments (11)
libs/blog/app-theme/data-access-app-theme/src/app-theme.store.ts (4)
3-3: Good addition of patchState import.The import of
patchStateis necessary for properly updating the signal store state in the new methods.
5-5: Good export of Theme type for reusability.Exporting the
Themetype allows other components to import and use the same type definition, promoting type consistency across the application.
27-31: Excellent implementation of theme toggle functionality.The
toggleThememethod correctly:
- Toggles between 'dark' and 'light' themes
- Updates the DOM attribute via the consumer
- Patches the store state with the new theme
The logic is clean and follows the signal store pattern properly.
45-47: Improvement: Using data attributes instead of CSS classes.The change from
setThemeClasstosetThemeAttributeis a good architectural improvement. Usingdata-themeattributes instead of CSS classes provides better separation of concerns and is more semantic for theme management.libs/blog/shell/feature-shell-web/src/lib/root-shell.component.ts (3)
79-81: Clean integration of theme store.The injection of
AppThemeStoreand creation of a computedthemesignal provides reactive theme state to the template. This follows Angular best practices for signal-based state management.
31-33: Proper template integration for theme management.The template correctly:
- Passes the reactive
theme()signal to the header component- Handles the
themeToggleevent to trigger theme switchingThis creates a clean data flow from store → shell → header.
104-106: Simple and effective theme toggle handler.The
onThemeTogglemethod correctly delegates to the store'stoggleThememethod, maintaining separation of concerns between UI events and state management.libs/blog/layouts/ui-layouts/src/lib/header/header.component.ts (4)
83-83: Well-typed theme input.The
themeinput is properly typed as'light' | 'dark'and marked as required, ensuring type safety and preventing undefined theme states.
87-87: Clean event-driven architecture.The
themeToggleoutput event follows Angular patterns for component communication, allowing the parent component to handle theme changes appropriately.
91-93: Intuitive icon selection logic.The
themeSwitchIconcomputed property correctly shows:
- Moon icon in light mode (to indicate switching to dark)
- Sun icon in dark mode (to indicate switching to light)
This provides clear visual indication of the toggle action.
41-52: Excellent accessibility and styling for theme toggle.The theme toggle button implementation includes:
- Proper
aria-labelfor screen readers- Clean styling with transparent background
- Proper click event handling
- Test ID for automated testing (once typo is fixed)
- Semantic use of SVG icons
This provides a great user experience for theme switching.
libs/blog/layouts/ui-layouts/src/lib/header/header.component.ts
Outdated
Show resolved
Hide resolved
|
Deployed to dev environment |
|
PR is detected, will deploy to dev environment |
|
Deployed to dev environment |
|
PR is detected, will deploy to dev environment |
|
Deployed to dev environment |
|
PR is detected, will deploy to dev environment |
|
Deployed to dev environment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
tailwind.preset.js (1)
1-1: Remove unused import or utilize the imported themeVariants.The
themeVariantsimport from 'tailwindcss-theme-variants' is not being used in the plugins array. You're implementing a custom plugin instead.Apply this diff to remove the unused import:
-const { themeVariants } = require('tailwindcss-theme-variants');Alternatively, if you intend to use the library's implementation, replace the custom plugin with:
-const { themeVariants } = require('tailwindcss-theme-variants'); +const { themeVariants } = require('tailwindcss-theme-variants'); // In plugins array: - function ({ addVariant }) { - addVariant( - 'light', - '&:where([data-theme="light"], [data-theme="light"] *)', - ); - addVariant('dark', '&:where([data-theme="dark"], [data-theme="dark"] *)'); - }, + themeVariants({ + themes: { + light: 'selector', + dark: 'selector' + } + }),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
libs/blog/about-us/feature-about-us/src/lib/feature-about-us/feature-about-us.component.html(2 hunks)libs/blog/about-us/feature-about-us/src/lib/feature-about-us/feature-about-us.component.ts(2 hunks)libs/blog/articles/feature-latest-articles/src/lib/feature-latest-articles/feature-latest-articles.component.html(1 hunks)libs/blog/authors/ui-author-card/src/lib/author-card/author-card-template.component.ts(2 hunks)libs/blog/authors/ui-author-card/src/lib/author-card/author-card.component.html(2 hunks)libs/blog/authors/ui-author-card/src/lib/author-card/author-card.component.ts(1 hunks)libs/blog/layouts/ui-layouts/src/lib/footer/components/footer-social-media-icons.component.ts(1 hunks)libs/blog/shared/ui-button/src/lib/button/button.component.ts(1 hunks)libs/blog/shared/ui-card/src/lib/card.component.ts(2 hunks)libs/blog/shared/ui-social-media-icons/src/lib/social-media-icons.component.html(1 hunks)libs/blog/shared/ui-social-media-icons/src/lib/social-media-icons.component.ts(2 hunks)libs/shared/assets/src/lib/styles/main.scss(2 hunks)tailwind.preset.js(2 hunks)
✅ Files skipped from review due to trivial changes (2)
- libs/blog/articles/feature-latest-articles/src/lib/feature-latest-articles/feature-latest-articles.component.html
- libs/blog/authors/ui-author-card/src/lib/author-card/author-card.component.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- libs/blog/shared/ui-button/src/lib/button/button.component.ts
- libs/blog/layouts/ui-layouts/src/lib/footer/components/footer-social-media-icons.component.ts
- libs/blog/shared/ui-social-media-icons/src/lib/social-media-icons.component.html
- libs/blog/authors/ui-author-card/src/lib/author-card/author-card.component.html
- libs/blog/about-us/feature-about-us/src/lib/feature-about-us/feature-about-us.component.html
- libs/shared/assets/src/lib/styles/main.scss
- libs/blog/shared/ui-card/src/lib/card.component.ts
- libs/blog/authors/ui-author-card/src/lib/author-card/author-card-template.component.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
libs/blog/about-us/feature-about-us/src/lib/feature-about-us/feature-about-us.component.ts (1)
libs/blog/app-theme/data-access-app-theme/src/app-theme.store.ts (1)
AppThemeStore(11-38)
🔇 Additional comments (10)
libs/blog/shared/ui-social-media-icons/src/lib/social-media-icons.component.ts (3)
4-6: LGTM: Modern Angular signal importsGood adoption of Angular's signal-based reactive programming with
computedandinput.
15-15: LGTM: Well-defined variant typeThe type definition provides good type safety for the theming variants.
27-27: LGTM: Modern input signal usageUsing Angular's signal-based input with a sensible default value follows current best practices.
libs/blog/about-us/feature-about-us/src/lib/feature-about-us/feature-about-us.component.ts (3)
19-19: LGTM! Clean integration with the theme system.The import correctly adds the
AppThemeStorefollowing the established import pattern and module structure.
45-45: Excellent use of modern Angular patterns.The injection uses the proper
inject()function and correctly accesses the theme signal from the store. Thereadonlymodifier appropriately prevents direct modification.
47-47: Well-implemented theme-aware computed property.The computed property correctly implements the gradient hiding logic for light mode using Angular's reactive patterns. The naming is descriptive and the logic is straightforward and appropriate.
tailwind.preset.js (4)
5-5: LGTM: Appropriate darkMode configuration for attribute-based theming.The change from
['class']to'selector'correctly aligns with the data-theme attribute approach used throughout the application.
10-11: LGTM: New color token follows established pattern.The
al-primary-foregroundcolor token is properly defined using the CSS variable pattern with alpha value support.
19-19: LGTM: Grey color token addition supports theming system.The
al-greycolor token follows the established pattern and will be used in the updated box shadow configuration.
32-32: LGTM: Box shadow color update aligns with new color tokens.The change from
--borderto--greyCSS variable correctly utilizes the newly addedal-greycolor token.
libs/blog/shared/ui-social-media-icons/src/lib/social-media-icons.component.ts
Show resolved
Hide resolved
|
PR is detected, will deploy to dev environment |
|
Deployed to dev environment |
|
PR is detected, will deploy to dev environment |
|
Deployed to dev environment |
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
libs/blog/partners/ui-partners/src/lib/partners-list/partners-list.component.html (1)
7-10: Add rel="noopener noreferrer" to external linkPrevent reverse‑tabnabbing and block access to window.opener.
Apply this diff:
- target="_blank" + target="_blank" + rel="noopener noreferrer"
🧹 Nitpick comments (5)
libs/blog/partners/ui-partners/src/lib/partners-list/partners-list.component.html (2)
6-6: Outline with explicit color looks good; remove redundant classSwitch to light:outline with light:outline-al-border resolves prior feedback. light:rounded-lg is redundant since rounded-lg is already applied.
Apply this diff to simplify:
- class="light:outline light:outline-1 light:outline-al-border light:rounded-lg aspect-[2/1] w-full max-w-[160px] rounded-lg bg-white md:max-w-[200px] lg:transition-transform lg:hover:scale-105 lg:hover:cursor-pointer lg:motion-reduce:transition-none lg:motion-reduce:hover:scale-100" + class="light:outline light:outline-1 light:outline-al-border aspect-[2/1] w-full max-w-[160px] rounded-lg bg-white md:max-w-[200px] lg:transition-transform lg:hover:scale-105 lg:hover:cursor-pointer lg:motion-reduce:transition-none lg:motion-reduce:hover:scale-100"Also confirm the al-border token is exposed to outline color utilities in your Tailwind config.
12-17: Provide sizes for fill image to optimize and silence Angular warningsWith fill, specify sizes for better responsive selection.
Apply this diff:
- <img + <img class="object-scale-down p-1" [ngSrc]="partner.asset_URL" [alt]="partner.alt" fill + sizes="(min-width: 768px) 200px, 160px" />Jenkinsfile.deploy (1)
15-19: Make parameter intent explicit (expected format and constraints).Clarify that DB_MAIN_* must be Turso database names (not URLs), allowed chars [-_.a-zA-Z0-9], and must be either all set or all empty. This reduces misconfiguration and supports server-side validation below.
libs/shared/assets/src/lib/styles/main.scss (2)
8-25: Add color-scheme and ensure a default to avoid FOUC/unset variables
- Set color-scheme for the dark theme.
- Ensure a default theme is active (SSR or early inline script) or provide base :root fallbacks to prevent unstyled flash if [data-theme] is missing/late.
Apply within this block:
:root[data-theme='dark'] { + color-scheme: dark; --primary: 255 0 106; --foreground: 255 255 255; --primary-foreground: 255 255 255; --muted: 190 196 202; --border: 46 47 59; --card: 25 26 34; --background: 16 15 21; --roadmap-primary: 179 0 74; --roadmap-secondary: 102 0 43; --roadmap-accent: 72 28 171; --roadmap-hover-primary: 146 60 255; --roadmap-hover-secondary: 255 0 106; --roadmap-label-optional: 199 151 1; --roadmap-label-comingSoon: 0 139 56; --grey: 46 47 59; --footer-background: 25 26 34; }Optional base fallback (place before theme blocks):
/* Fallback defaults to avoid FOUC if [data-theme] is absent/late */ :root { color-scheme: light; --primary: 213 1 89; --foreground: 20 21 27; --primary-foreground: 0 0 0; --muted: 25 25 25; --border: 229 231 235; --card: 255 255 255; --background: 255 255 255; --grey: 241 241 241; --roadmap-primary: 179 0 74; --roadmap-secondary: 102 0 43; --roadmap-accent: 72 28 171; --roadmap-hover-primary: 146 60 255; --roadmap-hover-secondary: 255 0 106; --roadmap-label-optional: 25 26 34; --roadmap-label-comingSoon: 25 26 34; --footer-background: 254 254 254; }
99-103: Put.al-linkin components layer and add focus-visible; verify dark variant behavior
- Component class should live in @layer components for sane precedence.
- Add focus-visible affordance. If dark mode is driven by [data-theme], ensure Tailwind dark variant matches your selector or switch to token-based color.
Apply this diff:
-@layer base { +@layer components { .al-link { - @apply text-red-500 hover:underline dark:text-blue-400; + @apply text-red-500 hover:underline focus-visible:underline underline-offset-2 dark:text-blue-400; } }If dark mode is not configured to recognize [data-theme='dark'], either:
- switch to tokenized color (e.g., text-al-primary) instead of dark: variant, or
- configure Tailwind darkMode to use the data-theme selector.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
Jenkinsfile.deploy(3 hunks)apps/blog/scripts/build-routes.mjs(1 hunks)libs/blog-bff/articles/api/src/lib/api.ts(4 hunks)libs/blog-bff/newsletter/src/lib/api.ts(0 hunks)libs/blog-bff/newsletter/src/lib/newsletter-client.ts(0 hunks)libs/blog-bff/shared/schema/src/lib/schema.ts(4 hunks)libs/blog/articles/ui-article-content/src/lib/article-content/article-content.component.scss(3 hunks)libs/blog/layouts/ui-layouts/src/lib/footer/components/footer-social-media-icons.component.ts(1 hunks)libs/blog/layouts/ui-layouts/src/lib/footer/footer.component.html(1 hunks)libs/blog/layouts/ui-navigation/src/lib/navigation/navigation.component.html(1 hunks)libs/blog/partners/ui-partners/src/lib/partners-list/partners-list.component.html(1 hunks)libs/shared/assets/src/lib/styles/main.scss(3 hunks)tailwind.preset.js(3 hunks)
💤 Files with no reviewable changes (2)
- libs/blog-bff/newsletter/src/lib/newsletter-client.ts
- libs/blog-bff/newsletter/src/lib/api.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- libs/blog/articles/ui-article-content/src/lib/article-content/article-content.component.scss
- libs/blog/layouts/ui-navigation/src/lib/navigation/navigation.component.html
- tailwind.preset.js
- libs/blog/layouts/ui-layouts/src/lib/footer/footer.component.html
🧰 Additional context used
🧬 Code graph analysis (3)
apps/blog/scripts/build-routes.mjs (1)
libs/blog/articles/data-access/src/lib/infrastructure/articles.service.ts (2)
ArticlesService(12-44)getArticleList(28-37)
libs/blog/layouts/ui-layouts/src/lib/footer/components/footer-social-media-icons.component.ts (1)
libs/blog/shared/ui-social-media-icons/src/lib/social-media-icons.component.ts (1)
SocialMediaIconsComponent(16-42)
libs/blog-bff/articles/api/src/lib/api.ts (1)
libs/blog-bff/shared/schema/src/lib/schema.ts (2)
articles(52-144)articleCounts(146-169)
🔇 Additional comments (6)
libs/blog/layouts/ui-layouts/src/lib/footer/components/footer-social-media-icons.component.ts (2)
12-12: LGTM: Theming token applied.The hardcoded color has been replaced with a design token (
text-al-foreground). This differs from the previously suggestedtext-al-primary-foreground, buttext-al-foregroundis semantically appropriate for footer text unless the footer has a primary-colored background requiring contrast adjustment.
16-16: LGTM: Explicit variant usage.The explicit
variant="default"aligns with the updated SocialMediaIconsComponent API. While the default value makes this redundant, being explicit improves code clarity.libs/blog-bff/articles/api/src/lib/api.ts (1)
54-54: LGTM: showHiddenFilter correctly applied.The
showHiddenFilterhelper and its usage in the list and count queries correctly implement the hidden-article filtering logic. WhenshowHiddenis present (even without a value), hidden articles are included; otherwise, they are excluded by default.Also applies to: 69-69, 146-151
libs/blog-bff/shared/schema/src/lib/schema.ts (2)
104-104: LGTM: indexes correctly updated to includeisHidden.All article-related covering indexes and the articleCounts unique constraint have been correctly updated to include
isHidden, which will maintain index efficiency for queries that filter by hidden status.Also applies to: 111-111, 118-118, 125-125, 132-132, 139-139, 166-166
77-77: Verify or add migration for non-nullisHidden.
No migration found addingis_hidden; add a migration that creates the column with a default (e.g.false) and backfills existing rows.libs/shared/assets/src/lib/styles/main.scss (1)
27-44: Light theme: add native color‐scheme and DRY shared roadmap tokens
- Inside
:root[data-theme='light'], addcolor-scheme: light;for native form controls and scrollbars.- Move common roadmap variables (
--roadmap-primary,--roadmap-secondary,--roadmap-accent,--roadmap-hover-primary,--roadmap-hover-secondary) to global:root, overriding only--roadmap-label-optionaland--roadmap-label-comingSoonper theme.- No
darkModesetting found intailwind.config.js; verify Tailwind is configured to use an attribute strategy ondata-theme="dark".
|
PR is detected, will deploy to dev environment |
|
Deployed to dev environment |
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
tailwind.preset.js (1)
5-5: InvaliddarkModeconfiguration persists.The previous review correctly flagged that
darkMode: 'selector'is invalid. In Tailwind CSS 3.4+, this value must be one of:
'media'(uses prefers-color-scheme)'class'(uses.darkclass)['selector', '<selector>']tuple (e.g.,['selector', '[data-theme="dark"]'])Since you're using
data-themeattributes (lines 59, 61), apply:- darkMode: 'selector', + darkMode: ['selector', '[data-theme="dark"]'],However, this creates a conflict with your custom
darkvariant at line 61. See next comment.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
libs/blog/app-theme/data-access-app-theme/src/app-theme.store.ts(2 hunks)libs/blog/roadmap/ui-roadmap-node/src/lib/components/roadmap-basic-node/roadmap-basic-node.component.ts(2 hunks)libs/shared/assets/src/lib/styles/main.scss(3 hunks)tailwind.preset.js(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- libs/blog/app-theme/data-access-app-theme/src/app-theme.store.ts
- libs/blog/roadmap/ui-roadmap-node/src/lib/components/roadmap-basic-node/roadmap-basic-node.component.ts
🔇 Additional comments (1)
libs/shared/assets/src/lib/styles/main.scss (1)
8-44: Theme tokens well-structured, but missing default fallback.The theme-scoped CSS custom properties are correctly organized with appropriate values for both light and dark modes. Token naming is consistent with the Tailwind preset color definitions.
However, if
data-themeis not set on the root element (e.g., during initial page load before JS runs), none of these variables will be defined, potentially causing rendering issues.Add a default theme block to handle the unattributed state:
@layer base { + :root { + /* Default to dark theme values for initial render */ + --primary: 255 0 106; + --foreground: 255 255 255; + /* ... copy all dark theme tokens ... */ + } + :root[data-theme='dark'] { --primary: 255 0 106;Alternatively, verify that your theme store sets
data-themesynchronously during app initialization before the first paint.
| @layer base { | ||
| .al-link { | ||
| @apply text-blue-400 hover:underline; | ||
| @apply text-red-500 hover:underline dark:text-blue-400; | ||
| } |
There was a problem hiding this comment.
dark: variant will not work with current preset configuration.
The dark:text-blue-400 utility relies on Tailwind's built-in dark: variant, which requires a valid darkMode configuration in tailwind.preset.js. Currently, that file has an invalid darkMode: 'selector' setting (see review comments on that file), so this dark: variant will not be generated.
After fixing the darkMode config in tailwind.preset.js, this will work correctly. If you choose to remove the built-in dark: variant entirely and only use custom dark variants, update this line to use the custom variant:
- @apply text-red-500 hover:underline dark:text-blue-400;
+ @apply text-red-500 hover:underline data-[theme=dark]:text-blue-400;Or use the custom dark variant from the preset (if properly implemented):
.al-link {
@apply text-red-500 hover:underline;
@apply dark:text-blue-400;
}This depends on the resolution of the tailwind.preset.js issues.
🤖 Prompt for AI Agents
In libs/shared/assets/src/lib/styles/main.scss lines 99-102, the use of the
built-in Tailwind dark: variant (dark:text-blue-400) will not be generated
because tailwind.preset.js has an invalid darkMode config; fix the preset to
enable the built-in dark variant (e.g., set darkMode: 'class' or 'media', or
correctly configure the selector-based dark mode per Tailwind docs) so
dark:text-blue-400 is produced, OR remove reliance on the built-in variant and
update this rule to first apply the base utilities then apply the custom dark
variant (e.g., keep @apply text-red-500 hover:underline; then @apply
dark:text-blue-400;) but only if your preset implements a custom dark variant —
choose one of these two fixes and apply it in tailwind.preset.js or this SCSS
accordingly.
| plugins: [ | ||
| function ({ addVariant }) { | ||
| addVariant( | ||
| 'light', | ||
| '&:where([data-theme="light"], [data-theme="light"] *)', | ||
| ); | ||
| addVariant('dark', '&:where([data-theme="dark"], [data-theme="dark"] *)'); | ||
| }, | ||
| ], |
There was a problem hiding this comment.
Redundant custom variants conflict with built-in dark: mode.
You manually register light and dark variants, but Tailwind already provides a built-in dark: variant via the darkMode config. This creates overlapping functionality and potential specificity conflicts.
Choose one approach:
Option 1 (Recommended): Use Tailwind's built-in dark: and a custom light: variant:
- darkMode: 'selector',
+ darkMode: ['selector', '[data-theme="dark"]'],
plugins: [
function ({ addVariant }) {
addVariant('light', '&:where([data-theme="light"], [data-theme="light"] *)');
- addVariant('dark', '&:where([data-theme="dark"], [data-theme="dark"] *)');
},
]Option 2: Use the imported themeVariants plugin (removes manual implementation):
plugins: [
- function ({ addVariant }) {
- addVariant('light', '&:where([data-theme="light"], [data-theme="light"] *)');
- addVariant('dark', '&:where([data-theme="dark"], [data-theme="dark"] *)');
- },
+ themeVariants({
+ themes: {
+ light: { selector: '[data-theme="light"]' },
+ dark: { selector: '[data-theme="dark"]' }
+ }
+ })
]If using Option 2, remove the darkMode config entirely to avoid conflicts. Based on learnings.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| plugins: [ | |
| function ({ addVariant }) { | |
| addVariant( | |
| 'light', | |
| '&:where([data-theme="light"], [data-theme="light"] *)', | |
| ); | |
| addVariant('dark', '&:where([data-theme="dark"], [data-theme="dark"] *)'); | |
| }, | |
| ], | |
| // In tailwind.preset.js | |
| // Replace the invalid selector string with a proper tuple for built-in dark: | |
| darkMode: ['selector', '[data-theme="dark"]'], | |
| plugins: [ | |
| function ({ addVariant }) { | |
| // keep only the custom light: variant | |
| addVariant( | |
| 'light', | |
| '&:where([data-theme="light"], [data-theme="light"] *)', | |
| ); | |
| - addVariant('dark', '&:where([data-theme="dark"], [data-theme="dark"] *)'); | |
| }, | |
| ], |
🤖 Prompt for AI Agents
In tailwind.preset.js around lines 55 to 63, you register custom 'light' and
'dark' variants that overlap Tailwind's built-in dark: mode; remove the
redundant manual 'dark' variant and either (a) keep Tailwind's darkMode and only
add a custom 'light' variant (recommended), or (b) if you prefer the
themeVariants plugin, remove the manual addVariant calls and delete the darkMode
config to let themeVariants handle both light/dark. Update the file accordingly
so there is only one source of truth for dark handling and no conflicting
variants.
|
PR is detected, will deploy to dev environment |
|
Deployed to dev environment |
|
PR is detected, will deploy to dev environment |
|
Deployed to dev environment |
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (5)
libs/blog/articles/ui-article-card/src/lib/components/article-compact-card/article-compact-card.component.html (3)
3-3: Theme token instead of hard-coded blackReplace bg-black with a theme-aware token (e.g., bg-al-card) to support light/dark modes.
6-13: Absolute image fit/positioning and boolean priorityUse object-fit utilities and explicit positioning (e.g., absolute inset-0 h-full w-full object-cover) and coerce priority to boolean: [priority]="!!imagePriority()".
14-14: Text color should use theme token; ensure stackingPrefer text-al-foreground (or equivalent) over text-[#fff] and ensure content has higher z-index than overlay/image.
tailwind.preset.js (2)
53-61: Remove redundant custom dark variant or align with darkModeAvoid overlapping Tailwind’s built-in dark:. Either keep darkMode and only add a custom light: variant, or adopt themeVariants plugin and drop darkMode/manual variants.
3-3: Fix darkMode configurationdarkMode: 'selector' is not valid alone. Use darkMode: ['selector', '[data-theme="dark"]'] or revert to 'class'/'media'.
🧹 Nitpick comments (2)
libs/blog/shared/ui-button/src/lib/button/button.component.ts (1)
24-24: Use theme token for text color instead of hard-coded whitePrefer text-al-primary-foreground to match the new tokenized theming.
Apply:
- Primary: 'bg-al-primary/90 text-white uppercase', + Primary: 'bg-al-primary/90 text-al-primary-foreground uppercase',libs/blog/articles/ui-article-card/src/lib/components/article-compact-card/article-compact-card.component.ts (1)
24-24: Clarify priority typing vs NgOptimizedImage booleanNgOptimizedImage [priority] expects boolean; imagePriority is number|null. Ensure template coerces to boolean (e.g., [priority]="!!imagePriority()") or change the input type to boolean if that matches usage across variants.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
apps/blog/src/assets/i18n/en.json(1 hunks)apps/blog/src/assets/i18n/pl.json(1 hunks)libs/blog/articles/ui-article-card/src/lib/components/article-compact-card/article-compact-card.component.html(1 hunks)libs/blog/articles/ui-article-card/src/lib/components/article-compact-card/article-compact-card.component.ts(2 hunks)libs/blog/authors/ui-author-card/src/lib/author-card/author-card-template.component.ts(2 hunks)libs/blog/authors/ui-author-card/src/lib/author-card/author-card.component.ts(1 hunks)libs/blog/layouts/ui-layouts/src/lib/header/header.component.ts(4 hunks)libs/blog/layouts/ui-navigation/src/lib/navigation/navigation.component.html(1 hunks)libs/blog/layouts/ui-navigation/src/lib/navigation/navigation.component.ts(2 hunks)libs/blog/newsletter/feature-newsletter/src/lib/page/newsletter-examples/newsletter-examples.component.ts(0 hunks)libs/blog/roadmap/ui-roadmap-dialog/src/lib/roadmap-dialog-additional-description/roadmap-dialog-additional-description.component.ts(1 hunks)libs/blog/roadmap/ui-roadmap/src/lib/components/vertical-connector-arrow/vertical-connector-arrow.component.ts(1 hunks)libs/blog/search/feature-search/src/lib/feature-search/global-search.component.ts(1 hunks)libs/blog/shared/ui-button/src/lib/button/button.component.ts(1 hunks)tailwind.preset.js(3 hunks)
💤 Files with no reviewable changes (1)
- libs/blog/newsletter/feature-newsletter/src/lib/page/newsletter-examples/newsletter-examples.component.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- libs/blog/roadmap/ui-roadmap-dialog/src/lib/roadmap-dialog-additional-description/roadmap-dialog-additional-description.component.ts
- libs/blog/layouts/ui-navigation/src/lib/navigation/navigation.component.html
- libs/blog/roadmap/ui-roadmap/src/lib/components/vertical-connector-arrow/vertical-connector-arrow.component.ts
- libs/blog/layouts/ui-layouts/src/lib/header/header.component.ts
🧰 Additional context used
🧬 Code graph analysis (3)
libs/blog/authors/ui-author-card/src/lib/author-card/author-card-template.component.ts (2)
libs/blog/articles/ui-article-card/src/lib/ui-article-card/ui-article-card.component.ts (1)
UiArticleCardComponent(26-30)libs/blog/shared/ui-card/src/lib/card.component.ts (2)
GradientCardDirective(69-74)CardComponent(94-101)
libs/blog/articles/ui-article-card/src/lib/components/article-compact-card/article-compact-card.component.ts (5)
libs/blog/shared/types/src/lib/article-card.ts (1)
ArticleCard(1-13)libs/blog/articles/ui-article-card/src/lib/components/article-regular-card/article-regular-card.component.ts (1)
ArticleRegularCardComponent(22-25)libs/blog/articles/ui-article-card/src/lib/ui-article-card/ui-article-card.component.ts (1)
UiArticleCardComponent(26-30)libs/blog/shared/ui-avatar/src/lib/avatar.component.ts (1)
AvatarComponent(13-17)libs/blog/articles/ui-article-card/src/lib/components/article-horizontal-card/article-horizontal-card.component.ts (1)
ArticleHorizontalCardComponent(36-52)
libs/blog/articles/ui-article-card/src/lib/components/article-compact-card/article-compact-card.component.html (2)
libs/blog/articles/ui-article-card/src/lib/components/article-regular-card/article-regular-card.component.ts (1)
ArticleRegularCardComponent(22-25)libs/blog/articles/ui-article-card/src/lib/ui-article-card/ui-article-card.component.ts (1)
UiArticleCardComponent(26-30)
🔇 Additional comments (10)
libs/blog/search/feature-search/src/lib/feature-search/global-search.component.ts (1)
17-17: LGTM! Typo fixed.The attribute name has been corrected from
date-testidtodata-testid, aligning with the HTML5 data attribute convention and ensuring test selectors function properly.libs/blog/authors/ui-author-card/src/lib/author-card/author-card-template.component.ts (3)
1-1: LGTM!The added imports (
computed,input) are correctly used throughout the component for defining signals and computed properties.
19-32: LGTM!The template structure correctly uses dynamic class bindings with computed properties to adjust layout based on the
articleCardinput. The responsive design patterns withmd:breakpoints are appropriate.
16-16: GradientCardDirective already defineshideGradientas an input
The directive in libs/blog/shared/ui-card/src/lib/card.component.ts (line 73) useshideGradient = input<boolean>(false), so the[hideGradient]binding is valid.libs/blog/authors/ui-author-card/src/lib/author-card/author-card.component.ts (1)
47-48: LGTM!The new
hideGradientandarticleCardinputs are correctly implemented using Angular's signal-based input API. The default values are appropriate for the parent component context.libs/blog/shared/ui-button/src/lib/button/button.component.ts (1)
20-20: Alignment improvement looks goodAdding justify-center is a sensible default for consistent horizontal alignment.
apps/blog/src/assets/i18n/pl.json (1)
31-31: Translation addition LGTMtoggle_theme key added and aligned with EN.
Confirm it’s used for visible label and aria-label/title in the theme toggle control.
apps/blog/src/assets/i18n/en.json (1)
31-31: Translation addition LGTMKey is well placed under nav and aligns with PL.
libs/blog/articles/ui-article-card/src/lib/components/article-compact-card/article-compact-card.component.ts (1)
13-19: NgOptimizedImage integration looks goodIncluding NgOptimizedImage in imports is appropriate for optimized loading.
tailwind.preset.js (1)
11-12: CSS variables exist; al-roadmap-gradient is static
CSS custom properties--primary-foreground,--grey, and--footer-backgroundare defined in libs/shared/assets/src/lib/styles/main.scss;al-roadmap-gradientuses a hardcoded radial gradient and doesn’t require a CSS var.
| readonly hideGradient = input<boolean>(true); | ||
| readonly articleCard = input<boolean>(false); |
There was a problem hiding this comment.
Inconsistent default values between parent and template components.
The hideGradient input defaults to true here but defaults to false in the parent AuthorCardComponent (line 47 of author-card.component.ts). This mismatch could lead to unexpected behavior when the parent doesn't explicitly pass the value.
Align the defaults to match the parent component:
- readonly hideGradient = input<boolean>(true);
+ readonly hideGradient = input<boolean>(false);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| readonly hideGradient = input<boolean>(true); | |
| readonly articleCard = input<boolean>(false); | |
| readonly hideGradient = input<boolean>(false); | |
| readonly articleCard = input<boolean>(false); |
🤖 Prompt for AI Agents
In
libs/blog/authors/ui-author-card/src/lib/author-card/author-card-template.component.ts
around lines 41 to 42, the hideGradient input currently defaults to true which
mismatches the parent AuthorCardComponent (author-card.component.ts line 47)
that defaults it to false; change the hideGradient input default to false so
both components share the same default behavior, and verify there are no other
input default mismatches between the template and parent.
| protected readonly authorInfoCardClass = computed(() => | ||
| !this.articleCard() | ||
| ? 'md:border dark:!bg-al-radial-gradient dark:bg-al-background md:light:bg-[#f2f2f2] pb-6' | ||
| : '', | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Verify Tailwind configuration for light mode variant and avoid !important.
Line 50 contains two potential issues:
- The
!importantflag indark:!bg-al-radial-gradientmight indicate a specificity issue that should be resolved through proper CSS architecture rather than forcing precedence. - The
md:light:bg-[#f2f2f2]uses alight:variant that may not be a standard Tailwind feature. Verify this variant is properly configured in your Tailwind setup.
Run the following script to check your Tailwind configuration:
Additionally, search the web to confirm whether light: is a valid Tailwind CSS variant in your version:
🏁 Script executed:
#!/bin/bash
# Description: Check Tailwind configuration for light mode variant
# Search for Tailwind config files
fd -e js -e ts 'tailwind.config'
# Search for light mode variant configuration
rg -nP --type=ts --type=js -C5 'variant.*light|darkMode|class.*strategy'
# Check if light: is used elsewhere in the codebase
rg -nP --type=ts --type=html 'light:' -g '!node_modules'Length of output: 1856
🌐 Web query:
Is "light:" a valid Tailwind CSS variant for light mode styling? How should light mode be implemented with Tailwind's dark mode class strategy?
💡 Result:
Short answer: No — Tailwind does not include a built‑in light: variant. Tailwind exposes a dark: variant (and the unprefixed classes act as the “light” / default styles). Configure dark mode with darkMode: 'media' or darkMode: 'class' and toggle by adding/removing a .dark parent (commonly on ). Example:
-
tailwind.config.js
- darkMode: 'class' // or 'media'. [1]
-
HTML usage (class strategy)
-
…
If you want an explicit light: variant, add a custom variant (Tailwind v4 supports @custom-variant) or use a selector like html:not(.dark) with an arbitrary selector utility. Example custom variant pattern shown in community answers:
- @custom-variant light (html:not(.dark) &); /* then use light:bg-red-500 */. [2]
Sources: Tailwind dark mode docs and community examples. [1][2]
References:
[1] Tailwind CSS — Dark Mode docs.
[2] StackOverflow: “TailwindCSS: How to do a light mode only modification?”
Avoid !important and define or remove light: variant
Remove the ! in dark:!bg-al-radial-gradient and handle specificity via Tailwind utilities or CSS. Since Tailwind has no built-in light: variant, add a custom light variant in your config (e.g., using @custom-variant or html:not(.dark)) or revert to unprefixed classes for light mode.
🤖 Prompt for AI Agents
In
libs/blog/authors/ui-author-card/src/lib/author-card/author-card-template.component.ts
around lines 48 to 52, remove the `!` (important) from
`dark:!bg-al-radial-gradient` and replace it with a normal
`dark:bg-al-radial-gradient` (or refactor to use a higher-specificity
utility/CSS rule) and either remove the unsupported `light:` prefix from the
light-mode class or add a custom `light` variant in the Tailwind config (e.g.,
define a `light` variant using `html:not(.dark)` or `@custom-variant`) so
light-mode styles are applied correctly; ensure specificity is handled via
Tailwind utilities or custom CSS rather than using `!important`.
libs/blog/layouts/ui-navigation/src/lib/navigation/navigation.component.ts
Outdated
Show resolved
Hide resolved
|
PR is detected, will deploy to dev environment |
|
Deployed to dev environment |
| return this.authorsCards()?.length || 0; | ||
| }); | ||
|
|
||
| readonly theme = inject(AppThemeStore).theme; |
libs/blog/articles/feature-comments/src/lib/giscus-comments/giscus-comments.component.ts
Show resolved
Hide resolved
|
PR is detected, will deploy to dev environment |
|
Deployed to dev environment |
| switch (this.variant()) { | ||
| case 'primary': | ||
| return 'm-[2px] bg-al-roadmap-primary text-[24px]'; | ||
| return 'm-[2px] bg-al-roadmap-primary light:bg-[#edcafa] text-[24px]'; |
There was a problem hiding this comment.
może coś zamiast light:bg-[#edcafa]
| }, | ||
| }) | ||
| export class GradientCardDirective { | ||
| hideGradient = input<boolean>(false); |
|
PR is detected, will deploy to dev environment |
|
Deployed to dev environment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
libs/blog/shared/ui-card/src/lib/card.component.ts (1)
103-103: Consider theming CardSkeletonComponent background
CardSkeletonComponent still usesbg-white(libs/blog/shared/ui-card/src/lib/card-skeleton.component.ts:38); consider applying theme-aware classes likebg-al-card/dark:bg-al-backgroundto match CardComponent.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
libs/blog/shared/ui-card/src/lib/card.component.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
libs/blog/shared/ui-card/src/lib/card.component.ts (1)
libs/blog/shared/ui-card/src/lib/card-skeleton.component.ts (1)
CardSkeletonComponent(34-39)
🔇 Additional comments (1)
libs/blog/shared/ui-card/src/lib/card.component.ts (1)
68-76: LGTM! Clean implementation of conditional gradient control.The refactoring correctly implements the gradient visibility feature:
- The
hideGradientinput with proper signal syntax and default value- Dynamic class binding (
!hideGradient()) applies the gradient by default- Simplified
hostClassesto only theme-specific styling (dark:bg-al-background)
Readded light-mode branch in order to have the possibility to test it out.
Summary by CodeRabbit
New Features
Style
Chores